BlobLTTIntegrator extended with added support for expanding blob radius - #203
BlobLTTIntegrator extended with added support for expanding blob radius#203grzegorzbor wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #203 +/- ##
==========================================
+ Coverage 97.18% 97.19% +0.01%
==========================================
Files 52 52
Lines 5045 5071 +26
==========================================
+ Hits 4903 4929 +26
Misses 142 142 ☔ View full report in Codecov by Harness. |
7e4e2bb to
4c487f4
Compare
| ] | ||
| assert u.allclose(u.Quantity(widths), 2 * (1e16 * u.cm / c).to("s"), rtol=1e-12) | ||
|
|
||
| def test_expanding_window_has_asymmetric_limits(self): |
There was a problem hiding this comment.
as we discussed today this test is very weak (it only checks sign) instead it is much better to include a test in which min and max times are calculated by hand and compared with the output
| ``start_time`` rather than at 0. When ``False``, that situation raises instead, naming | ||
| how much earlier ``blob``'s history would need to start. | ||
| ``start_time`` rather than at 0. Its radius is not an assumption, though: under | ||
| ``expansion`` the backdated snapshot's ``R_b`` is set to ``integrator.radius_at(start)``, |
There was a problem hiding this comment.
I do not think you really need this special case. If the blob is expanding assuming that it has a steady state is hard to justify. You can just throw an error if somebody switches on this option with expanding blob
jsitarek
left a comment
jsitarek
left a comment
There was a problem hiding this comment.
Ok, just some of the descriptions are to be clarified
…us - documentation improvements
21727cf to
1558b0e
Compare
2 participants
No description provided.